New lint: `unnecessary_map_or` #11796

Jacherr · 2023-11-12T19:43:40Z

This lint checks map_or method calls to check if they can be consolidated down to something simpler and/or more readable.

For example, the code

let x = Some(5);
x.map_or(false, |n| n == 5)

can be rewritten as

let x = Some(5);
x == Some(5)

In addition, when the closure is more complex, the code can be altered from, say,

let x = Ok::<Vec<i32>, i32>(vec![5]);
x.map_or(false, |n| n == [5])

into

let x = Ok::<Vec<i32>, i32>(vec![5]);
x.is_some_and(|n| n == [5])

This lint also considers cases where the map_or can be chained with other method calls, and accommodates accordingly by adding extra parentheses as needed to the suggestion.

changelog: add new lint unnecessary_map_or

rustbot · 2023-11-12T19:43:45Z

r? @Centri3

(rustbot has picked a reviewer for you, use r? to override)

clippy_lints/src/methods/unnecessary_map_or.rs

WaffleLapkin · 2024-03-02T16:09:18Z

@Jacherr this seems to be waiting on you, since Catherine left you a review.
@rustbot author

Jacherr · 2024-03-07T12:42:18Z

Hi, apologies for the lack of movement on this. I’ll make changes based on the review now, and once everything is up to standard I’ll go through the conflicts.

Jacherr · 2024-03-08T11:22:33Z

@rustbot ready

bors · 2024-03-09T10:09:43Z

☔ The latest upstream changes (presumably #12310) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2024-03-22T16:28:53Z

☔ The latest upstream changes (presumably #12507) made this pull request unmergeable. Please resolve the merge conflicts.

xFrednet · 2024-06-20T20:07:18Z

Hey @Jacherr Sorry for the delay. @Centri3 was sadly busy, but now she indicated that she'll have time for reviews again. Would you mind rebasing again? Next time there is a long delay, you can also pick a new reviewer with r? clippy.

bors · 2024-06-27T17:02:25Z

☔ The latest upstream changes (presumably #12999) made this pull request unmergeable. Please resolve the merge conflicts.

Centri3

Just a couple more changes then this is good <3

tests/ui/unnecessary_map_or.rs

clippy_lints/src/methods/unnecessary_map_or.rs

xFrednet · 2024-07-02T10:02:22Z

Never mind: My GH view was outdated

xFrednet · 2024-07-23T06:43:52Z

Hey, this is a ping from triage. @Centri3 can you give this PR a review? It's totally fine if you don't have the time right now, you can reassign the PR to a random team member using r? clippy.

@rustbot ready

bors · 2024-07-27T03:25:07Z

☔ The latest upstream changes (presumably #13168) made this pull request unmergeable. Please resolve the merge conflicts.

clippy_lints/src/methods/unnecessary_map_or.rs

bors · 2024-10-07T15:45:55Z

☔ The latest upstream changes (presumably #13496) made this pull request unmergeable. Please resolve the merge conflicts.

Jacherr · 2024-10-17T21:01:59Z

Apologies for the slow turnover again. I hope this is the last of the issues now and this can finally be merged 😅

samueltardieu · 2024-10-17T22:08:12Z

It looks like you marked the comment in #11796 (comment) as resolved but it is not. The example:

fn main() {
    struct S;
    let r: Result<i32, S> = Ok(3);
    let _ = r.map_or(false, |x| x == 7);
}

will incorrectly suggest using (r == Ok(7)) (also, note the extra parentheses, but this is not the main problem, which is that Result<i32, S> does not implement PartialEq). A proposed fix is given in the linked comment.

Could you also add this as a testcase? As well as one where S is a type which implements PartialEq (such as i32), to check for the extra parentheses?

samueltardieu · 2024-10-17T22:09:58Z

tests/ui/unnecessary_map_or.fixed

-    let _ = Some(5) == Some(5);
-    let _ = Some(5) != Some(5);
-    let _ = Some(5) == Some(5);
+    let _ = (Some(5) == Some(5));


These ( should be removed, shouldn't they?

I think this is just an effect of maybe_par deciding they should be present. While I'd usually agree, I'm not sure there is a better way to tackle that?

maybe_par is rather overeager, but unused_parens will usually catch them anyway (outside of arithmetic, for readability). New lints triggering on fixed code in a cascade is fine

samueltardieu · 2024-10-29T06:13:31Z

Is it time to open a FCP for this lint? I keep seeing .map_or(false, …) used everywhere.

bors · 2024-10-29T08:12:37Z

☔ The latest upstream changes (presumably #13437) made this pull request unmergeable. Please resolve the merge conflicts.

Centri3 · 2024-11-03T09:04:30Z

Opened the FCP. Can you squash?

Jacherr · 2024-11-03T16:34:30Z

Squash done, ready for final comments/changes then hopefully can finally get this merged. Apologies again for how long this took.

samueltardieu · 2024-11-04T18:34:58Z

@Jacherr It looks like .map_or(true, …) can be linted as well, using .is_none_or(…). I played with it a bit but I have to go AFK right now, so feel free to integrate the top commit from https://github.com/samueltardieu/rust-clippy/commits/push-oqltkmvvzmlq/ in your code if you wish, and update all the lints which trigger this code. Otherwise, I'll submit a complete PR after yours is merged.

Note that tests that don't involve PartialEq are machine applicable as far as I can see, so I've changed this as well, but not looked closely at why the others aren't.

Centri3 · 2024-11-04T22:08:18Z

I am happy with merging this as-is if no concerns come up, and having that done later.

samueltardieu · 2024-11-04T22:59:10Z

I am happy with merging this as-is if no concerns come up, and having that done later.

This seems reasonable indeed.

bors · 2024-11-07T18:11:03Z

☔ The latest upstream changes (presumably #13639) made this pull request unmergeable. Please resolve the merge conflicts.

samueltardieu · 2024-11-07T19:11:37Z

@Jacherr Commit 430235e is an updated version of your commit compatible with the current master

samueltardieu · 2024-11-07T19:36:24Z

(an alternative is to merge #13653 which contains this PR, updated, and the .map_or(true, …) case as well)

Jacherr · 2024-11-12T22:59:51Z

Hopefully, everything is in order for merge now. Ideally, as mentioned in the FCP, this can be merged and the additional map_or(true case added in post. It would be nice, at least for me, to put this PR to bed properly given how long it's been going back and forth.

Centri3 · 2024-11-13T00:08:58Z

Thanks for your work on this :) I doubt anybody will object the lint being added so let's put it to rest ❤️

rustbot assigned Centri3 Nov 12, 2023

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 12, 2023

Centri3 reviewed Nov 19, 2023

View reviewed changes

y21 mentioned this pull request Nov 21, 2023

Replace map_or(false, ...) with is_some_and or is_ok_and #11848

Open

rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 2, 2024

Jacherr force-pushed the pattern-match-partialeq branch from ad74a63 to abd23dc Compare March 7, 2024 15:33

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Mar 8, 2024

Jacherr force-pushed the pattern-match-partialeq branch from 2acf442 to 857081a Compare March 9, 2024 12:39

This comment was marked as resolved.

Sign in to view

Jacherr force-pushed the pattern-match-partialeq branch from 2f923e6 to c8341ac Compare June 22, 2024 11:26

Centri3 approved these changes Jun 28, 2024

View reviewed changes

Jacherr force-pushed the pattern-match-partialeq branch from 686975d to d305884 Compare June 28, 2024 22:33

Centri3 reviewed Aug 9, 2024

View reviewed changes

ronnodas mentioned this pull request Sep 11, 2024

clippy::option_if_let_else suggests hard code #8829

Open

y21 mentioned this pull request Oct 6, 2024

Detect .map_or(false, f) in manual_is_variant_and lint #13509

Closed

Jacherr force-pushed the pattern-match-partialeq branch from c558bf5 to a25b386 Compare October 6, 2024 19:31

Jacherr force-pushed the pattern-match-partialeq branch from 35ad982 to c150da9 Compare October 17, 2024 20:35

samueltardieu reviewed Oct 17, 2024

View reviewed changes

Jacherr force-pushed the pattern-match-partialeq branch 2 times, most recently from 5e9ca40 to fc78116 Compare November 3, 2024 16:29

samueltardieu mentioned this pull request Nov 4, 2024

Rustup #13639

Merged

samueltardieu mentioned this pull request Nov 4, 2024

Handle Option::map_or(true, …) in unnecessary_map_or lint #13653

Merged

Jacherr force-pushed the pattern-match-partialeq branch from fc78116 to ca5b59a Compare November 12, 2024 22:30

new lint unnecessary_map_or

89210d7

Jacherr force-pushed the pattern-match-partialeq branch from 0e440e1 to 89210d7 Compare November 12, 2024 23:00

samueltardieu approved these changes Nov 12, 2024

View reviewed changes

Centri3 added this pull request to the merge queue Nov 13, 2024

Merged via the queue into rust-lang:master with commit b829d53 Nov 13, 2024
9 checks passed

ctz mentioned this pull request Nov 25, 2024

Docs: unnecessary_map_or wrong "Added in" version #13730

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New lint: `unnecessary_map_or` #11796

New lint: `unnecessary_map_or` #11796

Jacherr commented Nov 12, 2023

rustbot commented Nov 12, 2023

WaffleLapkin commented Mar 2, 2024

Jacherr commented Mar 7, 2024

Jacherr commented Mar 8, 2024

bors commented Mar 9, 2024

bors commented Mar 22, 2024

xFrednet commented Jun 20, 2024

This comment was marked as resolved.

bors commented Jun 27, 2024

Centri3 left a comment

xFrednet commented Jul 2, 2024 •

edited

Loading

xFrednet commented Jul 23, 2024

bors commented Jul 27, 2024

bors commented Oct 7, 2024

Jacherr commented Oct 17, 2024 •

edited

Loading

samueltardieu commented Oct 17, 2024 •

edited

Loading

samueltardieu Oct 17, 2024

Jacherr Oct 17, 2024

Centri3 Oct 24, 2024 •

edited

Loading

samueltardieu commented Oct 29, 2024

bors commented Oct 29, 2024

Centri3 commented Nov 3, 2024

Jacherr commented Nov 3, 2024

samueltardieu commented Nov 4, 2024 •

edited

Loading

Centri3 commented Nov 4, 2024

samueltardieu commented Nov 4, 2024

bors commented Nov 7, 2024

samueltardieu commented Nov 7, 2024

samueltardieu commented Nov 7, 2024

Jacherr commented Nov 12, 2024 •

edited

Loading

Centri3 commented Nov 13, 2024

New lint: unnecessary_map_or #11796

New lint: unnecessary_map_or #11796

Conversation

Jacherr commented Nov 12, 2023

rustbot commented Nov 12, 2023

WaffleLapkin commented Mar 2, 2024

Jacherr commented Mar 7, 2024

Jacherr commented Mar 8, 2024

bors commented Mar 9, 2024

bors commented Mar 22, 2024

xFrednet commented Jun 20, 2024

This comment was marked as resolved.

bors commented Jun 27, 2024

Centri3 left a comment

Choose a reason for hiding this comment

xFrednet commented Jul 2, 2024 • edited Loading

xFrednet commented Jul 23, 2024

bors commented Jul 27, 2024

bors commented Oct 7, 2024

Jacherr commented Oct 17, 2024 • edited Loading

samueltardieu commented Oct 17, 2024 • edited Loading

samueltardieu Oct 17, 2024

Choose a reason for hiding this comment

Jacherr Oct 17, 2024

Choose a reason for hiding this comment

Centri3 Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

samueltardieu commented Oct 29, 2024

bors commented Oct 29, 2024

Centri3 commented Nov 3, 2024

Jacherr commented Nov 3, 2024

samueltardieu commented Nov 4, 2024 • edited Loading

Centri3 commented Nov 4, 2024

samueltardieu commented Nov 4, 2024

bors commented Nov 7, 2024

samueltardieu commented Nov 7, 2024

samueltardieu commented Nov 7, 2024

Jacherr commented Nov 12, 2024 • edited Loading

Centri3 commented Nov 13, 2024

New lint: `unnecessary_map_or` #11796

New lint: `unnecessary_map_or` #11796

xFrednet commented Jul 2, 2024 •

edited

Loading

Jacherr commented Oct 17, 2024 •

edited

Loading

samueltardieu commented Oct 17, 2024 •

edited

Loading

Centri3 Oct 24, 2024 •

edited

Loading

samueltardieu commented Nov 4, 2024 •

edited

Loading

Jacherr commented Nov 12, 2024 •

edited

Loading